fix: normalize --label flag from key=value to key:value for APISIX API#13
Conversation
APISIX Admin API expects label filters in key:value format (colon separator), but the CLI --label flag accepts key=value (equals). Add a shared NormalizeLabel function that converts between the two formats. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Route export and delete split "env=test" on "=" and sent only the key "env" to APISIX, which returned no results. Route list passed the label through without conversion. Use cmdutil.NormalizeLabel to send the correct "env:test" format. Remove broken client-side label filtering. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
… commands Same label normalization bug existed across all resource types. Apply cmdutil.NormalizeLabel consistently so --label env=test is correctly sent as label=env:test to the APISIX Admin API. Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
There was a problem hiding this comment.
Pull request overview
This PR fixes broken --label filtering across multiple CLI commands by normalizing user input from key=value to the APISIX Admin API’s expected key:value format and removing now-unneeded client-side label filtering in route bulk operations.
Changes:
- Added
cmdutil.NormalizeLabel()and unit tests to standardize--labelformatting. - Updated list/export/delete commands across multiple resources to pass normalized
labelquery parameters. - Simplified route export/delete by removing client-side label-value filtering and relying on API-side filtering.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/cmdutil/errors.go | Adds NormalizeLabel() helper used to convert key=value → key:value. |
| pkg/cmdutil/errors_test.go | Adds unit tests for NormalizeLabel(). |
| pkg/cmd/upstream/list/list.go | Normalizes --label before sending it as the label query param. |
| pkg/cmd/upstream/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/upstream/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/streamroute/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/streamroute/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/ssl/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/ssl/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/service/list/list.go | Normalizes --label before sending it as the label query param. |
| pkg/cmd/service/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/service/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/route/list/list.go | Normalizes --label before sending it as the label query param. |
| pkg/cmd/route/export/export.go | Removes client-side label filtering and sends normalized label query param. |
| pkg/cmd/route/export/export_test.go | Updates route export test to assert normalized label query format. |
| pkg/cmd/route/delete/delete.go | Removes client-side label filtering and sends normalized label query param. |
| pkg/cmd/proto/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/proto/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/pluginconfig/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/pluginconfig/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/globalrule/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/globalrule/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/consumergroup/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/consumergroup/delete/delete.go | Normalizes label query param during bulk-delete ID listing. |
| pkg/cmd/consumer/export/export.go | Normalizes label query param during paginated export fetch. |
| pkg/cmd/consumer/delete/delete.go | Normalizes label query param during bulk-delete listing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| parts := strings.SplitN(label, "=", 2) | ||
| if len(parts) == 2 { | ||
| return parts[0] + ":" + parts[1] | ||
| } | ||
| return label |
There was a problem hiding this comment.
NormalizeLabel currently rewrites any string containing "=" into "key:value" even when either side is empty (e.g., "env=" -> "env:", "=test" -> ":test"). That produces an invalid/ambiguous label query and makes it hard to surface a clear user error. Consider only normalizing when both key and value are non-empty (or trimming whitespace), and otherwise returning the original string (or changing the helper to return an error so callers can raise a FlagError).
| parts := strings.SplitN(label, "=", 2) | |
| if len(parts) == 2 { | |
| return parts[0] + ":" + parts[1] | |
| } | |
| return label | |
| parts := strings.SplitN(label, "=", 2) | |
| if len(parts) != 2 { | |
| return label | |
| } | |
| key := strings.TrimSpace(parts[0]) | |
| value := strings.TrimSpace(parts[1]) | |
| if key == "" || value == "" { | |
| // Leave invalid/ambiguous labels unchanged so callers can surface a clear error. | |
| return label | |
| } | |
| return key + ":" + value |
| for _, tc := range tests { | ||
| t.Run(tc.input, func(t *testing.T) { | ||
| assert.Equal(t, tc.expected, NormalizeLabel(tc.input)) | ||
| }) |
There was a problem hiding this comment.
This table-driven test uses tc.input as the subtest name; the empty-string case creates an unnamed subtest ("TestNormalizeLabel/") which is awkward to read and filter with -run. Consider using the test index or a formatted/quoted name (e.g., fmt.Sprintf(%q, tc.input)) so every subtest has a stable, non-empty name.
| func TestRouteExport_WithLabelFilter(t *testing.T) { | ||
| calledWithLabelKey := false | ||
| calledWithNormalizedLabel := false | ||
| transport := roundTripperFunc(func(req *http.Request) (*http.Response, error) { | ||
| if req.Method == http.MethodGet && req.URL.Path == "/apisix/admin/routes" { | ||
| if req.URL.Query().Get("label") == "env" { | ||
| calledWithLabelKey = true | ||
| if req.URL.Query().Get("label") == "env:test" { | ||
| calledWithNormalizedLabel = true | ||
| } | ||
| // Return routes with labels; client-side filtering will match env=test | ||
| return &http.Response{ | ||
| StatusCode: 200, | ||
| Header: http.Header{"Content-Type": []string{"application/json"}}, | ||
| Body: io.NopCloser(strings.NewReader(`{"total":2,"list":[{"key":"/apisix/routes/r1","value":{"id":"r1","name":"match","uri":"/r1","labels":{"env":"test"}}},{"key":"/apisix/routes/r2","value":{"id":"r2","name":"no-match","uri":"/r2","labels":{"env":"prod"}}}]}`)), | ||
| Body: io.NopCloser(strings.NewReader(`{"total":1,"list":[{"key":"/apisix/routes/r1","value":{"id":"r1","name":"match","uri":"/r1","labels":{"env":"test"}}}]}`)), | ||
| }, nil |
There was a problem hiding this comment.
The label-filter test doesn't actually validate filtering behavior: the mock server always returns only the matching route, regardless of what label query was sent. That means the test would still pass even if the command ignored --label entirely (as long as the query string check flips the boolean). Consider having the round tripper return different bodies depending on the label query (filtered vs unfiltered) and re-adding an assertion that non-matching routes are not present in the output.
Summary
--label key=valuefilters acrosslist,export, anddeletecommands were broken. APISIX Admin API expectskey:value(colon separator), but the CLI sent either just the key or the rawkey=valuestring, causing empty results.cmdutil.NormalizeLabel()that convertskey=value→key:value, applied consistently across all 26 affected files (route, upstream, service, consumer, ssl, consumergroup, globalrule, proto, streamroute, pluginconfig).Test
NormalizeLabelcovering empty, equals, colon, key-only, and multi-equals casesenv:testquery parameter formatmake checkpasses (only pre-existingTestFindAsset_MatchCurrentPlatformfailure unrelated to this change)